New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ignore non-integer ranges in Style/RandomWithOffset #8754
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it wouldn't be better to mark this cop as unsafe instead of restricting it like this.
I'm a bit confused as to which cases are actually problematic...
@@ -36,15 +36,15 @@ class RandomWithOffset < Base | |||
(send | |||
{nil? (const {nil? cbase} :Random) (const {nil? cbase} :Kernel)} | |||
:rand | |||
{int irange erange})) | |||
{int (irange (int _) (int _)) (erange (int _) (int _))})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No biggie, but (int _)
=> int
is simpler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated!
2d5b2e0
to
be1a55e
Compare
The previous behaviour was a crash, so I don't think marking as unsafe will cut it:
This happens because the range bounds are assumed to be integer literals here later: It might be possible to extend the cop to accept non-integer bounds (perhaps unsafely), but I think that can be a followup. |
This cop only works correctly for ranges with integer bounds.
be1a55e
to
32fdcaf
Compare
Cool, thanks! |
This cop only works correctly for ranges with literal integer bounds.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.